Skip to content

Conversation

@XChy
Copy link
Member

@XChy XChy commented Oct 21, 2024

@XChy XChy requested review from dtcxzyw and goldsteinn October 21, 2024 13:39
@XChy XChy requested a review from nikic as a code owner October 21, 2024 13:39
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-llvm-transforms

Author: XChy (XChy)

Changes

Fixes #113123
Alive proof: https://alive2.llvm.org/ce/z/hnqeLC


Full diff: https://github.com/llvm/llvm-project/pull/113164.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (+4-2)
  • (modified) llvm/test/Transforms/InstCombine/fcmp-range-check-idiom.ll (+39)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index ed9a89b14efcca..678149f56eb901 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -1444,8 +1444,10 @@ Value *InstCombinerImpl::foldLogicOfFCmps(FCmpInst *LHS, FCmpInst *RHS,
     }
     if (IsLessThanOrLessEqual(IsAnd ? PredL : PredR)) {
       BuilderTy::FastMathFlagGuard Guard(Builder);
-      Builder.setFastMathFlags(LHS->getFastMathFlags() |
-                               RHS->getFastMathFlags());
+      FastMathFlags NewFlag = LHS->getFastMathFlags();
+      if (!IsLogicalSelect)
+        NewFlag |= RHS->getFastMathFlags();
+      Builder.setFastMathFlags(NewFlag);
 
       Value *FAbs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, LHS0);
       return Builder.CreateFCmp(PredL, FAbs,
diff --git a/llvm/test/Transforms/InstCombine/fcmp-range-check-idiom.ll b/llvm/test/Transforms/InstCombine/fcmp-range-check-idiom.ll
index 10a3ccf3cdb48a..54dbb09cb8fd31 100644
--- a/llvm/test/Transforms/InstCombine/fcmp-range-check-idiom.ll
+++ b/llvm/test/Transforms/InstCombine/fcmp-range-check-idiom.ll
@@ -359,3 +359,42 @@ define i1 @test_and_olt_fmf_propagation_union(float %x) {
   %cond = and i1 %cmp1, %cmp2
   ret i1 %cond
 }
+
+define i1 @test_and_olt_fmf_propagation_union_logical_rhs_poison(float %x) {
+; CHECK-LABEL: define i1 @test_and_olt_fmf_propagation_union_logical_rhs_poison(
+; CHECK-SAME: float [[X:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call float @llvm.fabs.f32(float [[X]])
+; CHECK-NEXT:    [[COND:%.*]] = fcmp olt float [[TMP1]], 0x3C00000000000000
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %cmp1 = fcmp ninf olt float %x, 0x3C00000000000000
+  %cmp2 = fcmp ogt float %x, 0xBC00000000000000
+  %cond = select i1 %cmp2, i1 %cmp1, i1 false
+  ret i1 %cond
+}
+
+define i1 @test_and_olt_fmf_propagation_union_logical_lhs_poison(float %x) {
+; CHECK-LABEL: define i1 @test_and_olt_fmf_propagation_union_logical_lhs_poison(
+; CHECK-SAME: float [[X:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call ninf float @llvm.fabs.f32(float [[X]])
+; CHECK-NEXT:    [[COND:%.*]] = fcmp ninf olt float [[TMP1]], 0x3C00000000000000
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %cmp1 = fcmp olt float %x, 0x3C00000000000000
+  %cmp2 = fcmp ninf ogt float %x, 0xBC00000000000000
+  %cond = select i1 %cmp2, i1 %cmp1, i1 false
+  ret i1 %cond
+}
+
+define i1 @test_and_olt_fmf_propagation_union_logical_both_poison(float %x) {
+; CHECK-LABEL: define i1 @test_and_olt_fmf_propagation_union_logical_both_poison(
+; CHECK-SAME: float [[X:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call ninf float @llvm.fabs.f32(float [[X]])
+; CHECK-NEXT:    [[COND:%.*]] = fcmp ninf olt float [[TMP1]], 0x3C00000000000000
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %cmp1 = fcmp ninf olt float %x, 0x3C00000000000000
+  %cmp2 = fcmp ninf ogt float %x, 0xBC00000000000000
+  %cond = select i1 %cmp2, i1 %cmp1, i1 false
+  ret i1 %cond
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@XChy XChy merged commit a2ba438 into llvm:main Oct 21, 2024
6 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong canonicalization of a range check idiom

3 participants